feat(http/unstable): add RFC 9421 message signatures#7039
feat(http/unstable): add RFC 9421 message signatures#7039tomas-zijdemans wants to merge 12 commits intodenoland:mainfrom
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #7039 +/- ##
==========================================
+ Coverage 94.43% 94.47% +0.04%
==========================================
Files 630 631 +1
Lines 50566 51170 +604
Branches 8969 9164 +195
==========================================
+ Hits 47750 48345 +595
- Misses 2247 2249 +2
- Partials 569 576 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
bartlomieju
left a comment
bartlomieju
left a comment
There was a problem hiding this comment.
Thanks for the thorough implementation! A few things to address:
badge.svg changes don't belong in this PR — The reformatting of the SVG is unrelated to message signatures. Please drop it from the branch (or submit separately).
|
T%hanks for the feedback!
|
bartlomieju
left a comment
bartlomieju
left a comment
There was a problem hiding this comment.
Previous round's feedback is fully addressed (ECDSA/HMAC/RSA-PSS throws, ;bs docs, @query-param RFC citation). This is a solid RFC 9421 implementation — well-structured, thorough tests, good use of the existing structured fields module.
A couple of things I'd like addressed before approving:
| // expose getAll(). Splitting on ", " is therefore the best we can do, but | ||
| // it will mishandle single header values that contain a literal ", " (e.g. | ||
| // the Date header). Avoid using ;bs on such fields. | ||
| const values = headerValue.split(", "); |
There was a problem hiding this comment.
The ;sf resolution tries Dictionary → List → Item. Tests cover Dictionary and Item fallback paths, but there's no test for a header that fails Dictionary parse but succeeds as a List (e.g. a header like "1, 2, 3" or "(a b), (c d)"). That List branch is untested — please add a test case.
| * @param message The HTTP request or response to verify. | ||
| * @param keyLookup Resolves a key identifier to a CryptoKey, or `null` if the | ||
| * key is not found. When the signature has no `keyid` parameter, the empty | ||
| * string `""` is passed. |
There was a problem hiding this comment.
If the message already has a signature with the same label (e.g. default "sig"), appendHeader will produce a Signature-Input dictionary with duplicate keys. Per RFC 8941, the last value wins on parse, so the old signature is silently replaced while its byte sequence lingers in the Signature header.
Consider either:
- Checking for label conflicts and throwing, or
- Documenting in
SignatureParams.labelthat the caller is responsible for uniqueness
The multi-signature test uses distinct labels and works correctly, so this only matters for the default-label case with pre-signed messages.
2 participants
Add HTTP Message Signatures (RFC 9421) for signing and verifying HTTP requests and responses. Builds on the structured fields module (RFC 8941) already in std.
Coverage is as good as I can get it. There are a few lines of unreachable code.